Skip to content

IFC-2275 Add Jinja2 filters for artifact composition#889

Open
gmazoyer wants to merge 12 commits intoinfrahub-developfrom
gma/infp-504-artifact-composition
Open

IFC-2275 Add Jinja2 filters for artifact composition#889
gmazoyer wants to merge 12 commits intoinfrahub-developfrom
gma/infp-504-artifact-composition

Conversation

@gmazoyer
Copy link
Contributor

@gmazoyer gmazoyer commented Mar 24, 2026

Why

Customers building modular configuration pipelines need to compose larger artifacts from smaller sub-artifacts inside Jinja2 transforms, without duplicating template logic or GraphQL query fields. Today there is no way for a template to reference the rendered content of another artifact.

This PR implements the SDK side of the "Artifact of Artifacts" initiative, adding new Jinja2 filters and the supporting infrastructure to enable artifact content composition.

What changed

  • artifact_content that accepts a storage_id string, fetches artifact content from the object store via ObjectStore.get()
  • file_object_content with the same pattern but uses the /api/files/by-storage-id/{storage_id} endpoint, with binary content-type rejection
  • from_json / from_yaml to parse strings into Python dicts/lists, enabling chaining like {{ sid | artifact_content | from_json }}
  • Replaced FilterDefinition.trusted: bool with allowed_contexts: ExecutionContext using Python's Flag enum
  • Three contexts: CORE (API server computed attributes, most restrictive), WORKER (Prefect workers), LOCAL (CLI/unrestricted)
  • New optional client: InfrahubClient constructor parameter
  • New set_client() method for deferred client injection (per PR Add plan to implement INFP-504 #885 feedback)
  • Without a client, client-dependent filters raise JinjaFilterError with an actionable message
  • JinjaFilterError(filter_name, message, hint) subclassing JinjaTemplateError
  • New get_file_by_storage_id() method (async + sync) using /api/files/by-storage-id/{storage_id}
  • Content-type validation: rejects non-text responses (application/octet-stream, etc.)

Suggested review order

  1. infrahub_sdk/template/filters.py for ExecutionContext enum and FilterDefinition migration
  2. infrahub_sdk/template/infrahub_filters.py for InfrahubFilters class, from_json, from_yaml
    3 infrahub_sdk/template/__init__.py for Jinja2Template wiring, set_client(), validate() changes
  3. infrahub_sdk/object_store.py for get_file_by_storage_id()

How to review

  • validate() context logic iit is the core of the trust model change
  • _register_client_filters() and set_client() ensure client-dependent filters are correctly bound or fallback

How to test

uv run pytest tests/unit/sdk/test_infrahub_filters.py -v
uv run pytest tests/unit/sdk/test_template.py -v
uv run pytest tests/unit/sdk/test_object_store.py -v

Impact & rollout

  • Backward compatibility: validate(restricted=True/False) continues to work as before. FilterDefinition.trusted property preserved. Jinja2Template constructor is backward compatible (new client param is optional with default None).
  • Server-side change: required but it is a part of this body of work

Checklist

  • Tests added/updated
  • Changelog entry added (uv run towncrier create ...)
  • External docs updated (if user-facing or ops-facing change)
  • Internal .md docs updated (internal knowledge and AI code tools knowledge)

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced template filters for retrieving artifact and file content
    • Added from_json and from_yaml filters for template data transformation
    • Enabled deferred client configuration for templates
    • Expanded filter execution context support for broader template compatibility
  • Documentation

    • Updated specification with new artifact composition features and validation model
  • Tests

    • Added comprehensive test coverage for template filters and validation

@coderabbitai
Copy link

coderabbitai bot commented Mar 24, 2026

Walkthrough

This pull request introduces Jinja2 template filters for artifact composition within the Infrahub SDK. It adds four new filters: artifact_content and file_object_content (which retrieve content from storage), and from_json and from_yaml (which parse string content). The implementation includes a new ExecutionContext flag-based system that replaces the previous boolean trusted field in filter definitions, enabling context-aware template validation. The Jinja2Template class now accepts an optional client parameter and includes a new set_client() method for deferred client injection. Storage file retrieval is enabled through a new get_file_by_storage_id() method in the object store. A new exception type JinjaFilterError provides filter-specific error reporting. Comprehensive unit tests validate the new filters and validation behavior.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding Jinja2 filters for artifact composition, which is the core objective of the PR.
Description check ✅ Passed The description covers all key sections: Why, What changed, How to review, How to test, Impact & rollout, and Checklist. It provides sufficient detail and context for reviewers.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Mar 24, 2026

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 06fda33
Status: ✅  Deploy successful!
Preview URL: https://5dcbf956.infrahub-sdk-python.pages.dev
Branch Preview URL: https://gma-infp-504-artifact-compos.infrahub-sdk-python.pages.dev

View logs

@gmazoyer gmazoyer force-pushed the gma/infp-504-artifact-composition branch from ce7a42e to d7514e6 Compare March 24, 2026 08:47
@codecov
Copy link

codecov bot commented Mar 24, 2026

Codecov Report

❌ Patch coverage is 79.60526% with 31 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
infrahub_sdk/object_store.py 46.80% 23 Missing and 2 partials ⚠️
infrahub_sdk/template/__init__.py 86.66% 1 Missing and 3 partials ⚠️
infrahub_sdk/template/infrahub_filters.py 96.36% 1 Missing and 1 partial ⚠️
@@                 Coverage Diff                  @@
##           infrahub-develop     #889      +/-   ##
====================================================
+ Coverage             80.98%   81.04%   +0.05%     
====================================================
  Files                   120      121       +1     
  Lines                 10449    10591     +142     
  Branches               1562     1580      +18     
====================================================
+ Hits                   8462     8583     +121     
- Misses                 1475     1489      +14     
- Partials                512      519       +7     
Flag Coverage Δ
integration-tests 39.44% <0.00%> (-0.54%) ⬇️
python-3.10 52.65% <54.60%> (+0.12%) ⬆️
python-3.11 52.65% <54.60%> (+0.12%) ⬆️
python-3.12 52.65% <54.60%> (+0.12%) ⬆️
python-3.13 52.65% <54.60%> (+0.10%) ⬆️
python-3.14 54.35% <56.29%> (+0.12%) ⬆️
python-filler-3.12 23.82% <31.57%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/template/exceptions.py 96.96% <100.00%> (+0.96%) ⬆️
infrahub_sdk/template/filters.py 100.00% <100.00%> (ø)
infrahub_sdk/template/infrahub_filters.py 96.36% <96.36%> (ø)
infrahub_sdk/template/__init__.py 94.70% <86.66%> (-2.20%) ⬇️
infrahub_sdk/object_store.py 59.09% <46.80%> (+2.62%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gmazoyer gmazoyer marked this pull request as ready for review March 24, 2026 10:45
@gmazoyer gmazoyer requested a review from a team as a code owner March 24, 2026 10:45
@gmazoyer gmazoyer force-pushed the gma-20260323-infp504-plan branch from 56476d3 to 4e2de5e Compare March 25, 2026 08:17
Base automatically changed from gma-20260323-infp504-plan to infrahub-develop March 25, 2026 08:28
@gmazoyer gmazoyer force-pushed the gma/infp-504-artifact-composition branch from 28ec56e to 75800b8 Compare March 25, 2026 08:35
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
dev/specs/infp-504-artifact-composition/contracts/filter-interfaces.md (1)

80-87: Add blank lines before and after the bullet list.

As per coding guidelines for markdown files, add blank lines before and after lists.

📝 Suggested fix
 **Accepted content-types** (text-based):
+
 - `text/*`
 - `application/json`
 - `application/yaml`
 - `application/x-yaml`
+
 **Rejected**: All other content-types → `JinjaFilterError` with binary content message.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@dev/specs/infp-504-artifact-composition/contracts/filter-interfaces.md`
around lines 80 - 87, Add blank lines immediately before the "Accepted
content-types (text-based):" heading and immediately after the bullet list
(before the "Rejected:" line) so the list is separated by empty lines per
markdown guidelines; locate the block containing "Accepted content-types
(text-based):" and the following bullets (`text/*`, `application/json`,
`application/yaml`, `application/x-yaml`) and insert one blank line above and
one blank line below that list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@dev/specs/infp-504-artifact-composition/contracts/filter-interfaces.md`:
- Line 22: Update the validation context text to include LOCAL for both
artifact_content and file_object_content: change "Allowed in WORKER context" to
"Allowed in WORKER and LOCAL contexts" (or equivalent) to match the code where
artifact_content and file_object_content have
allowed_contexts=ExecutionContext.WORKER | ExecutionContext.LOCAL in
infrahub_sdk/template/filters.py; ensure the Validation line for both entries
mentions LOCAL alongside WORKER and that CORE remains as Blocked.

In `@dev/specs/infp-504-artifact-composition/data-model.md`:
- Around line 156-157: Update the FilterDefinition registrations for
"artifact_content" and "file_object_content" to reflect the actual allowed
contexts by changing allowed_contexts from ExecutionContext.WORKER to
ExecutionContext.WORKER | ExecutionContext.LOCAL (matching the implementation in
infrahub_sdk/template/filters.py), and update the accompanying comment from
"worker context only" to "worker and local contexts"; ensure you modify the
FilterDefinition(...) entries for those two names and any nearby comment text in
the same file.

In `@dev/specs/infp-504-artifact-composition/spec.md`:
- Line 102: The spec FR-007 is inconsistent with the code: the filters
artifact_content and file_object_content are registered in FilterDefinition with
allowed_contexts=ExecutionContext.WORKER | ExecutionContext.LOCAL in
infrahub_sdk/template/filters.py; update the spec text for FR-007 to list both
WORKER and LOCAL (or explicitly state that LOCAL is allowed alongside WORKER) so
the documented allowed_contexts matches the implementation for
FilterDefinition/validate().

---

Nitpick comments:
In `@dev/specs/infp-504-artifact-composition/contracts/filter-interfaces.md`:
- Around line 80-87: Add blank lines immediately before the "Accepted
content-types (text-based):" heading and immediately after the bullet list
(before the "Rejected:" line) so the list is separated by empty lines per
markdown guidelines; locate the block containing "Accepted content-types
(text-based):" and the following bullets (`text/*`, `application/json`,
`application/yaml`, `application/x-yaml`) and insert one blank line above and
one blank line below that list.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 52ef8c23-d67e-4725-952d-62d9bb6240bd

📥 Commits

Reviewing files that changed from the base of the PR and between aecc708 and 75800b8.

📒 Files selected for processing (15)
  • changelog/+artifact-composition.added.md
  • changelog/+artifact-composition.changed.md
  • dev/specs/infp-504-artifact-composition/contracts/filter-interfaces.md
  • dev/specs/infp-504-artifact-composition/data-model.md
  • dev/specs/infp-504-artifact-composition/plan.md
  • dev/specs/infp-504-artifact-composition/quickstart.md
  • dev/specs/infp-504-artifact-composition/research.md
  • dev/specs/infp-504-artifact-composition/spec.md
  • dev/specs/infp-504-artifact-composition/tasks.md
  • infrahub_sdk/object_store.py
  • infrahub_sdk/template/__init__.py
  • infrahub_sdk/template/exceptions.py
  • infrahub_sdk/template/filters.py
  • infrahub_sdk/template/infrahub_filters.py
  • tests/unit/sdk/test_infrahub_filters.py

@gmazoyer gmazoyer force-pushed the gma/infp-504-artifact-composition branch from 75800b8 to 06fda33 Compare March 25, 2026 08:52
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
infrahub_sdk/template/infrahub_filters.py (1)

102-113: Type annotation doesn't cover scalar YAML values.

The return type dict | list doesn't account for valid YAML scalar values (e.g., yaml.safe_load("42") returns 42). The spec documentation at filter-interfaces.md line 64 states the output is "Parsed dict, list, or scalar". Consider updating the type hint for accuracy.

📝 Suggested fix
-def from_yaml(value: str) -> dict | list:
-    """Parse a YAML string into a Python dict or list."""
+def from_yaml(value: str) -> dict | list | str | int | float | bool | None:
+    """Parse a YAML string into a Python object (dict, list, or scalar)."""

Alternatively, use Any if the broader type is acceptable:

def from_yaml(value: str) -> Any:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@infrahub_sdk/template/infrahub_filters.py` around lines 102 - 113, The
from_yaml function's return type (dict | list) omits scalar YAML values; update
the type annotation on from_yaml to include scalars (e.g., dict | list | str |
int | float | bool | None) or use a broad Any to match yaml.safe_load and the
spec in filter-interfaces.md; keep the function body as-is and only change the
signature's return type to accurately reflect possible parsed outputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@infrahub_sdk/template/infrahub_filters.py`:
- Around line 102-113: The from_yaml function's return type (dict | list) omits
scalar YAML values; update the type annotation on from_yaml to include scalars
(e.g., dict | list | str | int | float | bool | None) or use a broad Any to
match yaml.safe_load and the spec in filter-interfaces.md; keep the function
body as-is and only change the signature's return type to accurately reflect
possible parsed outputs.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fdf43fec-51e1-4d3a-b048-ec9800eb6b30

📥 Commits

Reviewing files that changed from the base of the PR and between 75800b8 and 06fda33.

📒 Files selected for processing (12)
  • changelog/+artifact-composition.added.md
  • changelog/+artifact-composition.changed.md
  • dev/specs/infp-504-artifact-composition/contracts/filter-interfaces.md
  • dev/specs/infp-504-artifact-composition/data-model.md
  • dev/specs/infp-504-artifact-composition/spec.md
  • dev/specs/infp-504-artifact-composition/tasks.md
  • infrahub_sdk/object_store.py
  • infrahub_sdk/template/__init__.py
  • infrahub_sdk/template/exceptions.py
  • infrahub_sdk/template/filters.py
  • infrahub_sdk/template/infrahub_filters.py
  • tests/unit/sdk/test_infrahub_filters.py
✅ Files skipped from review due to trivial changes (3)
  • changelog/+artifact-composition.added.md
  • changelog/+artifact-composition.changed.md
  • infrahub_sdk/template/exceptions.py
🚧 Files skipped from review as they are similar to previous changes (4)
  • infrahub_sdk/object_store.py
  • dev/specs/infp-504-artifact-composition/data-model.md
  • dev/specs/infp-504-artifact-composition/tasks.md
  • dev/specs/infp-504-artifact-composition/spec.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants